-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #12593: Add support for BSD-style init scripts services (rc.d) (slackware) #764
Conversation
f63011b
to
f21b2c7
Compare
Commit modified |
f21b2c7
to
129a927
Compare
"destination_exists" and => { "destination_defined", "destination_not_empty" }; | ||
|
||
methods: | ||
"copy" usebundle => ncf_classes_copy("${source_prefix}", "${destination_prefix}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing indent
tree/20_cfe_basics/ncf_lib.cf
Outdated
@@ -987,6 +1017,10 @@ bundle agent ncf_services(service, action) | |||
"action_command" string => "${svc_action_cmd[${action}]}"; | |||
"method" string => "svcadm/svcs"; | |||
|
|||
# Slackware | |||
pass1.slackware.!systemctl_utility_present.!is_upstart_service.!svcadm_utility_present:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it is a slackware the other conditions are useless
"action_command" string => "${paths.path[test]} -f /etc/rc`/sbin/runlevel | ${paths.path[cut]} -d' ' -f2`.d/S??${service}"; | ||
"method" string => "/etc/rcX.d/"; | ||
|
||
# Slackware boot actions are not performed by command, they are implemented in the 'methods' promises |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this message doesn't seem to be at its place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is left here because in the previous "non-boot actions" block, we are defining action_commands and/or methods for all the platforms, so one may expect to find Slackware boot actions at this place. As we do not perform those actions in "vars" promises, this is explaining where to find them
ifvarclass => "!is_check_action"; | ||
# is-active is mapped to "status" | ||
"action_command" string => "/etc/rc.d/rc.${service} status", | ||
ifvarclass => "is_check_action"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should have been merged with the previous block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moreover the "method" string line should be here for a better readability
@@ -1114,7 +1158,7 @@ bundle agent ncf_services(service, action) | |||
##### | |||
# Actual command - for checks on non-windows systems | |||
##### | |||
pass1.is_check_action.!windows.!is_process_action.method_found:: | |||
pass1.is_check_action.!windows.!is_process_action.method_found.!(slackware.is_boot_action):: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can is_check_action be defined at the same time as is_boot_action ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, iff action == "is-enabled"
"chmod 644" usebundle => perm_on_file("/etc/rc.d/rc.${service}", "644", "${old_class_prefix}", "${class_prefix}"); | ||
|
||
action_enable:: | ||
"chmod 755" usebundle => perm_on_file("/etc/rc.d/rc.${service}", "755", "${old_class_prefix}", "${class_prefix}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idem
tree/20_cfe_basics/ncf_lib.cf
Outdated
action_is_enabled:: | ||
"enable dry run" usebundle => push_dry_run_mode("true"); | ||
"chmod 755" usebundle => perm_on_file("/etc/rc.d/rc.${service}", "755", "${canon_service}_init_script_execmod", "${canon_service}_init_script_execmod"); | ||
"block in init scripts" usebundle => service_block_rc_d("${service}", "${old_class_prefix}", "${class_prefix}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need an ifvarclass ${canon_service}_init_script_execmod_ok here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add it to force the bundle to stop checking on failing the first condition (755 mod on the init script).
It does work without it however, because in case it fails and then meets the "block in script" requirement, the _not_ok classes will still get reported.
tree/20_cfe_basics/ncf_lib.cf
Outdated
files: | ||
"/etc/rc.d/rc.local" | ||
edit_line => edit_service_block_rc_d("${service}"), | ||
classes => classes_generic("service_${canon_service}_block_in_rc_local"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we prefer editing rc.local over editing rc.M ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes !
tree/20_cfe_basics/ncf_lib.cf
Outdated
edit_line => edit_service_block_rc_d("${service}"), | ||
classes => classes_generic("service_${canon_service}_block_in_rc_local"); | ||
|
||
"/etc/rc.d/rc.M" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a line of comment to tell what happens when wry_run mode is enabled ?
ifvarclass => "ncf_services_${canonified_service_name}_is_enabled_reached"; | ||
"new result classes" usebundle => _classes_copy("${class_prefix}_check_enabled", "${class_prefix}"), | ||
ifvarclass => "${class_prefix}_check_enabled_reached"; | ||
"success" usebundle => _classes_success("${old_class_prefix}"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change ? is there a repaired that appears during an is-enabled ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was for testing purposes, I forgot to add it back.
This PR is not mergeable to upper versions. |
This PR is not mergeable to upper versions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a mistake to approve it
Commit modified |
129a927
to
8ce24f1
Compare
|
||
# When the dry run mode is enabled, the needed changes will not be applied, but the classes will be copied | ||
files: | ||
"/etc/rc.d/rc.local" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to manage rc.localshutdown too
Commit modified |
8ce24f1
to
9f87087
Compare
This PR is not mergeable to upper versions. |
OK, merging this PR |
https://www.rudder-project.org/redmine/issues/12593